Skip to content

Replace current expression.rs module with calls to the rust-miniscript's expression module - #112

Open
KyrylR wants to merge 5 commits into
ElementsProject:masterfrom
KyrylR:feature/upstream-expression
Open

Replace current expression.rs module with calls to the rust-miniscript's expression module#112
KyrylR wants to merge 5 commits into
ElementsProject:masterfrom
KyrylR:feature/upstream-expression

Conversation

@KyrylR

@KyrylR KyrylR commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

Built upon the #110

The #110 should be merged first

@KyrylR KyrylR mentioned this pull request Oct 8, 2026
Open
@KyrylR

KyrylR commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

See https://github.com/KyrylR/elements-miniscript/blob/298f2bc0eb8ef82876fe60fbe5f7e03a5a8258ff/CHANGELOG.md for the short description of changes

@KyrylR
KyrylR force-pushed the feature/upstream-expression branch 2 times, most recently from 20b073d to 2c5ed14 Compare October 8, 2026 14:50
@KyrylR
KyrylR force-pushed the feature/upstream-expression branch from 2c5ed14 to d65bcdd Compare October 26, 2026 10:01
@KyrylR
KyrylR marked this pull request as ready for review October 26, 2026 10:02
@KyrylR KyrylR mentioned this pull request Oct 29, 2026
Draft
@KyrylR
KyrylR force-pushed the feature/upstream-expression branch from d65bcdd to 4540de0 Compare October 29, 2026 12:34
@KyrylR

KyrylR commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

As discussed in the #108, this is a second step in revising simplicity descriptors

@apoelstra

apoelstra commented Oct 31, 2026

Copy link
Copy Markdown
Member

In 4bef5c4:

Fine to keep verify_checksum for now but we should remove it in a later PR and just use the upstream checksum computation. (The comment points out that they deviate because upstream uses the last # as a separator while we use the first. Upstream is correct, we should not preserve this difference.)

apoelstra
apoelstra reviewed Oct 31, 2026
View reviewed changes
Comment thread src/descriptor/mod.rs
// eltr is parsed without extensions first, and then with the extension
// type T. Tr::from_str will check the checksum
// match "tr(" to handle more extensibly
let desc = if s.starts_with(&format!("{}tr", ELMTS_STR)) {

@apoelstra apoelstra Oct 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 4bef5c4:

This is also okay to leave in for this PR but we should be able to rip out all these string-parsing and "try multiple parses in a row" junk.

@KyrylR

KyrylR commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Taking a look for both comments today

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@apoelstra apoelstra apoelstra left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@KyrylR @apoelstra